After some deliberation, I believe that adding dynamic player factions in this PR would make the initial factions implementation a little bit too ugly/large to merge in one go; it is a much more complicated system then what I originally expected. I'll make the earlier portions of the system ready to merge now, and the dynamic player factions implementation will be turned into an issue, to be resolved in a future PR.

Your branch seems to be behind the current master. In this state it won't compile with the current Economy module.

The branch has been updated with the lastest commits, it should compile correctly now 👍

Review

The PR does exactly as intended! Good job!
It adds a faction alignment component to the settlement and all the characters belonging to one faction are of that faction type.

The first thing I noticed was that the entire city spawns with just a single type of citizen (which is random). After some snooping and logging, I noticed that the SettlementRegisterEvent is received only once by the @RecieveEvent inside which resides the code for randomizing the faction alignment component.

Further, I was unable to test character aggression because of the lack of different types of citizens.

Another minor point is the the CitizenSpawnedEvent isn't really used. Maybe move some of the code to a method that receives that event.

Also I believe there might be some confusion regarding what settlements actually are. Are they the buildings that spawn or the entire city?. After glancing through the code of DynamicCities, it seems that Settlements are the cities themselves and the way it has been used in MR is probably for individual buildings. This confusion arose because of the name given to the parameter in the @RecieveEvent - (BuildingEntitySpawned). Maybe this should be changed so that buildings are given the components and every citizen spawning inside a building is given the particular alignment too.

Thank you for your review! First, for clarification, a settlement = a single city. A single city containing characters of only one faction type is intentional. This was done to show that a single city is "owned" by a particular faction. If this must to be changed to work better with your project this year, just let me know 😄

Faction aggression can be tested by spawning in characters using the command line. I'm going to add a testing guide for that now in the PR description.

You brought up a good point about using the CitizenSpawnedEvent, I'll update that when I can 👍 I have my final exams for university this week, so I may not be able to get to this until a bit later

I've added instructions for spawning characters to the PR description, just spawn two opposing characters next to each other and you should see the factions system in action!

I investigated moving the initialization of a citizen's faction to the FactionAlignmentSystem, and I found that this would be really difficult to do without a refactor of citizen prefabs. I'll do this in a future PR, but I don't want to do that at the moment, since this will mess with other currently made PRs.

Review

Nice Job! Finally got to see some good vs bad action. Once a bad and good Citizen were in a particular range, they both approached either and one of the gooeys disappeared.

It was nice to see that the Gooeys only attacked each other when they looked at each other.
However, the fact that beyond a particular range, the gooeys don't attack each other seems unrealistic. Maybe in the city, where there are walls, it might not seem too weird. It would be nice however to have a feature where the citizens would attack on sight (not completely sure how to implement this. As a stretch maybe even add a behaviour called ApproachOnSight or maybe just an event if it doesn't exist. Could be used for a ton of things like deer going to eat some plants or running away on seeing something dangerous).

The path finding again seems to be an issue here. (This one is really recurring isnt it ) Whenever the gooeys were on opposite sides of walls , or even on different heights of blocks, it just felt like the gooeys glitched when making the approach.

Even after quite some testing, I was unable to figure out what the criteria was which decided which Gooey would die. (I might have to go through the code). It would be nice to have it depend on the health of the Gooeys to make it more realistic.

Some animation would be nice too!

Thank you for looking at the PR again! I'll tackle each of the points you brought up in your comment one by one:

• I think scanning for enemies in a specified range works well for now. I'm afraid that tracing sightlines for many characters may add a large performance burden to the game!
• Yeah, the pathfinding issues pop up in many places. As soon as pathfinding is integrated for one part of the game, it will solve many issues like this.
• Currently fighting Gooeys do have health components which are used to determine which one survives the battle. The gooey with lower initial health usually wins the battle.
• I agree that animation would be fantastic, but that seems like something suited for a later PR (keeping in mind that these gooeys will be swapped out with models from Added good and bad robots assets #43)

As this pull request is currently in a functional state, I'm going to merge this now as is, and add any other improvements in a different pull request.